gz_waves: rendering - Part 3/4#884
Conversation
caguero
changed the title
62909c5 to
f4a163e
Compare
fb9d51b to
a2a41b5
Compare
f4a163e to
4d50173
Compare
a2a41b5 to
3b09c80
Compare
4d50173 to
7df415e
Compare
3b09c80 to
bb957dc
Compare
70b0854 to
69c71f1
Compare
509839f to
85be063
Compare
bb957dc to
2e79cc6
Compare
e1af261 to
8d6502e
Compare
…ridge Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Aguero <caguero@honurobotics.com>
…rx_bringup simulation.launch.xml` runs Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Aguero <caguero@honurobotics.com>
…bers HeightMapTexture::Upload now takes the raw column-major WaveField2D buffers + N and transposes to row-major internally (a plain loop, null channels -> zeros), so WaterVisual hands Field()'s pointers straight through with no Eigen::Map. Removes Eigen from the package (it was used only as a transpose helper and wasn't even declared as a dependency) and switches HeightMapTexture members to gz-style this->member. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
Extract the column-major -> row-major transpose from HeightMapTexture into a dependency-free GridReflow.hh and cover it with a gtest (the only data transform on the upload path, previously untested because it lived in an anonymous namespace and Upload needs a live Ogre scene). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
The analytic Gerstner engine writes the Jacobian determinant into the foam channel, but it barely leaves 1.0, so it carries no usable folding signal — whitecaps are visually indistinguishable from no foam. Gate foamStrength to 0 for the gerstner backend (the FS then skips the foam path entirely), matching the shader's documented intent; FFT/Encino keeps foam. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
- Fix a data race: hold mutex_ across ResolveVisual so its first-pass material setup (which calls UploadUniforms and reads the cross-thread cache) no longer races PreUpdate on the ECM thread. - Release the per-entity dedup claim in ~Implementation so a reloaded WaterVisual for the same entity can become active again. - Make the one-shot upload log a per-instance member, not a function static. - Tag sim as mutex_-guarded; note the deliberate lock over Update()/Upload(). - Sweep stale 'FFT' naming to 'grid/displacement' (the path is backend- agnostic), refresh foam doc, and explain the Configure const_cast. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
Drop the retired "backend" term in comments and Doxygen in favour of "engine", matching the vocabulary settled in the PR1 review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Aguero <caguero@honurobotics.com>
8d6502e to
e513a4d
Compare
| // for the periodic heightmap sample, so tile instances at | ||
| // different world offsets each render their own piece of the | ||
| // continuous wavefield (rather than each tile showing the same | ||
| // patch in local model space). The Gerstner VS doesn't declare |
There was a problem hiding this comment.
This comment says the Gerstner vertex shader doesn't declare world_matrix, which implies a second vertex shader exists. But this stack wires a single grid vertex shader (fft_water_vs_330.glsl) for every engine, and it does declare world_matrix. Is there still a separate Gerstner vertex shader, or is this comment vestigial from an earlier design where Gerstner had its own? If vestigial, the comment is misleading; if a second shader really exists, binding world_matrix unconditionally would throw on it — worth confirming which.
There was a problem hiding this comment.
Vestigial from the old design where each engine had its own vertex shader; there is no separate Gerstner VS now, so I deleted that sentence in dab473ec.
| // garbage from higher mips at distance — visible as fine dotted lines | ||
| // that drift with the waves. Matches `combinedTex` on the GPU path. | ||
| hm->texture->setNumMipmaps(1u); | ||
| // RGBA32F so we can pack (η, Dx, Dy, α) per texel. Alpha is unused for |
There was a problem hiding this comment.
This comment appears to be stale? Says the alpha channel is "unused for now (reserved … for a future stage)," but the very next function packs the foam/folding metric into alpha (waves_ogre2_heightmap_upload) and the fragment shader reads it (the useFoamMap path). Suggest updating the comment to describe alpha as the folding/foam channel.
There was a problem hiding this comment.
You are right, it is stale. Updated in dab473ec to describe alpha as the folding/foam channel packed in the upload path.
|
|
||
| uniform sampler2D bumpMap; | ||
| uniform samplerCube cubeMap; | ||
| // FFT heightmap. Only sampled when foamStrength > 0 (the gerstner |
There was a problem hiding this comment.
This says the heightMap sampler "can be unbound" for the Gerstner path since foamStrength=0. But the vertex shader always samples heightMap for the displacement (the Gerstner engine produces a Field() grid too), so the sampler is never actually unbound — only the fragment-shader foam read is skipped. Suggest clarifying so it doesn't imply the texture is optional.
There was a problem hiding this comment.
Clarified in dab473ec: the texture is always bound (the vertex shader samples it for displacement); only the fragment shader foam read is gated by foamStrength.
| float foam = ComputeFoamMask() * foamStrength; | ||
| color.rgb = mix(color.rgb, vec3(1.0), foam); | ||
|
|
||
| fragColor = vec4(color.xyz, 0.9); |
There was a problem hiding this comment.
Water alpha is hardcoded to 0.9 (vec4(color.xyz, 0.9)). Is the surface intended to be translucent, and is the material's blend/depth state configured for alpha blending? A constant 0.9 alpha can cause depth-sorting artifacts against other transparent objects; if opacity isn't intended, 1.0 avoids the blend path entirely. Should this be exposed as a user parameter?
There was a problem hiding this comment.
Set to opaque 1.0 in dab473ec to keep the water out of the transparent path (no blend state is configured); we can expose a translucency parameter later if wanted.
|
CC and I reviewed. Looking forward to the next PR with some full stack testing! Approved whenever @caguero is ready to merge. |
…paque Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Carlos Aguero <caguero@honurobotics.com>
2 participants
Third in the stacked series. Adds the renderer, the
WaterVisualGUI system that draws the wave surface, plus a runnable Gerstner-ocean demo world.How to test it?
What this PR adds
WaterVisual(gz-sim-water-visual-system) — a gz-sim GUI system that reads theWavefieldcomponent, rebuilds a thread-private engine from the replicated recipe via the core registry (CreateWaveSimulation), and uploads its height/displacement grid as a GPU heightmap each frame. It is provider-agnostic: it links no wave engine and depends only on thegz_wavescore.HeightMapTexture— the CPU→GPU heightmap helper.Ogre2HeightMapBridge(libwaves-ogre2-bridge) — all Ogre Next access is isolated behind a C-ABI in this separate library, loaded on demand viadlopen, soWaterVisualcarries noDT_NEEDEDonlibgz-rendering-ogre2(which would interfere with gz-rendering's own engine-plugin loader).water_surfacemodel (mesh + shaders + textures) — loadsWaterVisualplus the per-engine GUI registrar(s) in the same<visual>(the only level the GUI's GuiRunner loads systems at). At this point that isgz-sim-waves-gerstner-gui; the FFT registrar line is added by PR4.vrx_gazebo/worlds/open_water.sdf— wired as a Gerstner ocean (gerstner source +model://water_surface,<sea_state>default + advanced-commented block) sovrx_bringup's existing launch runs it directly. Ocean-only: no references outside the PR1–PR3 stack.Provider-agnostic design
The engine registration that lets
WaterVisualrebuild an engine in the GUI process lives in the per-engine GUI plugins (shipped by the provider packages), not in this package. So adding or removing an engine never touchesgz_waves_rendering— it depends only on the core.WaterVisualkeeps a private engine instance (not the component's) because the GUI render thread and the ECM thread both touch it; sharing would race.